Skip to content

fix(BA-6927): stop cross-multiplying usage bucket amounts and durations#12965

Draft
jopemachine wants to merge 1 commit into
mainfrom
fix/BA-6927-usage-bucket-cross-product
Draft

fix(BA-6927): stop cross-multiplying usage bucket amounts and durations#12965
jopemachine wants to merge 1 commit into
mainfrom
fix/BA-6927-usage-bucket-cross-product

Conversation

@jopemachine

@jopemachine jopemachine commented Jul 20, 2026

Copy link
Copy Markdown
Member

Summary

FairShareAggregator summed each bucket's resource amounts and slice durations
separately, then multiplied — computing (Σ amount) × (Σ duration) instead of
Σ(amount × duration). This inflates a bucket by the number of kernel slices
folded into it (≈ concurrent kernel count), and the error compounds up the
hierarchy. Reported case: one user with 4 GPU kernels for a day reads 1382400
fGPU-seconds instead of the correct 345600. kernel_usage_records, which
stores per-slice products, was never affected and is the rebuild source.

  • Aggregator now accumulates per-slice products, so the delta carries
    Σ(amount × duration) directly. BucketDelta (its only job was holding the
    two separate sums) collapses to a plain ResourceSlot.
  • usage_bucket_entries.amountresource_usage, dropping its
    NUMERIC(24,6) limit — a domain-level daily mem bucket runs to ~1e18
    byte-seconds; unbounded NUMERIC has no ceiling. Rename + typmod drop are
    metadata-only (no table rewrite). resource_usage is what the JSONB mirror and
    the three bucket tables already call this, so one term runs end to end.
  • duration_seconds dropped — nothing read it on its own; it only existed to
    reconstitute the product.
  • Read-path scale fix (second, independent defect): the fair-share read path
    summed the stored figure without multiplying by duration, so scheduling ran on
    a ~300× too-small scale. Storing the product directly corrects it.

Migration c4a91d7e05b2

Existing buckets are wrong in both the JSONB mirror and the normalized entries and
cannot be fixed in place, so both are rebuilt from kernel_usage_records:

  • Rebuilds usage_bucket_entries + the three *_usage_buckets JSONB mirrors.
    *_fair_shares is left alone — the next observer tick recomputes it from the
    corrected entries.
  • Excludes the oldest retained day (retention purges kernel records mid-day by
    period_end, so that day may be truncated). Buckets older than the
    kernel-record retention window keep their inflated values rather than being
    zeroed — unrecoverable, and zeroing would destroy the only history left. The
    ~28-day fair-share lookback sits inside the rebuildable window.
  • Rebuild is idempotent: it deletes the in-window entries and re-inserts from the
    immutable kernel_usage_records, and overwrites (not increments) the JSONB, so
    re-running yields the same values.
  • downgrade() reverts only the column definition (the rebuilt values are the
    correct ones; the inflated originals are unrecoverable) and logs a warning that
    the entries no longer match the restored schema.

The rebuild spans up to the retention window; back up before applying.

Test plan

  • pants test on the aggregator, resource_usage_history and retention suites —
    bucket assertions corrected to resource-seconds; regression parametrized over
    1/2/4/10 concurrent kernels; multi-tenant tick (two projects, one user active in
    both) pinning per-level factors and bucket keys
  • pants fmt/lint/check clean on the changeset
  • Migration verified on a live DB: down→up round-trips are idempotent, every
    in-window level rebuilds to its expected value, the oldest retained day stays
    untouched, buckets with no kernel records collapse to {}, and the downgrade
    warning prints
  • Verify on a live manager that new observation ticks accumulate correctly

Follow-ups (not in this PR)

  • The JSONB bucket update is a read-modify-write, not a server-side accumulate, so
    concurrent observers could lose an update (only leader election guards it today).
  • period_end is not in the on_conflict update set, so the period-extension
    strategy described in the bucket docstrings never actually happens.
  • ResourceSlot has no unit notion, so an allocation and an already-integrated
    value share a type — what made this defect expressible; distinguishing them
    properly spans ~15 files and belongs in its own change.

Resolves BA-6927

🤖 Generated with Claude Code

@github-actions github-actions Bot added size:XL 500~ LoC comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated labels Jul 20, 2026
jopemachine added a commit that referenced this pull request Jul 20, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jopemachine added a commit that referenced this pull request Jul 20, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jopemachine
jopemachine force-pushed the fix/BA-6927-usage-bucket-cross-product branch from baf0826 to 81f0067 Compare July 20, 2026 04:13
@jopemachine
jopemachine force-pushed the fix/BA-6927-usage-bucket-cross-product branch 15 times, most recently from 831d257 to 61f3350 Compare July 24, 2026 02:53
FairShareAggregator accumulated the raw resource amounts and the slice
durations of a bucket separately and multiplied them afterwards, yielding
(sum amount_k) * (sum duration_k) instead of sum(amount_k * duration_k).
The cross product inflates a bucket by the number of kernel slices folded
into it, so a user running 4 GPU kernels saw 4x their real fGPU-seconds.
The factor compounds up the hierarchy, since each level aggregates the
kernels of everything beneath it.

The deltas the aggregator hands over now carry the per-slice products
already summed, which is the only shape that fixes this: (sum amount, sum
duration) does not determine sum(amount * duration), so the pairing has to
survive into the delta. BucketDelta held exactly those two sums and had no
other purpose, so it collapses to a plain ResourceSlot per bucket.

usage_bucket_entries.amount becomes resource_usage, the name
kernel_usage_records, the three bucket tables and the GraphQL and REST
responses already use for this quantity, so one vocabulary runs the whole
way through. The column also drops its precision limit: no fixed precision
is defensible here -- a domain-level daily mem bucket runs to ~1e18
byte-seconds on a large cluster -- while PostgreSQL's unconstrained numeric
has no ceiling, and dropping a typmod does not rewrite the table.
duration_seconds goes too. It only ever existed so readers could
reconstitute the product, nothing consulted it on its own, and it counted
kernel-seconds rather than wall-clock, so keeping it would leave a column
that invites the same misreading.

The read paths previously summed the stored figure without combining it
with duration at all, returning something 300x smaller than the
resource-seconds the fair share calculator assumes (it divides by
capacity * lookback_days * 86400), so scheduling priorities were computed
on the wrong scale -- a second defect independent of the cross product.

Existing buckets cannot be corrected in place, in either the JSONB mirror
or the normalized entries, so the migration rebuilds both from
kernel_usage_records, which stores per-slice products and was never
affected. The oldest retained day is excluded because retention purges
kernel records by period_end and may have truncated it; buckets older than
the kernel record retention window keep their inflated values rather than
being silently zeroed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jopemachine
jopemachine force-pushed the fix/BA-6927-usage-bucket-cross-product branch from 61f3350 to b89eb6e Compare July 24, 2026 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:XL 500~ LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant